Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JIT: Fold const RVA access (via const index) #78783

Merged
merged 24 commits into from
Nov 29, 2022
Merged

Conversation

EgorBo
Copy link
Member

@EgorBo EgorBo commented Nov 23, 2022

Closes #64823

static ReadOnlySpan<byte> RVA => new byte[] { 1, 2, 3, 4, 5, 6, 7, 8 };

// Case1: read byte at a const position (2):
byte ReadByte() => RVA[2];


// Case2: Read Int32 at the give const offset (4)
int ReadInt() =>
    Unsafe.ReadUnaligned<int>(
        ref Unsafe.Add(ref MemoryMarshal.GetReference(RVA), 4));


// Case3: IsWhiteSpace is implemented using RVA table under its hood
bool IsWhiteSpace() => char.IsWhiteSpace(' ');

Previous codegen:

; Assembly listing for method Prog:ReadByte():ubyte:this
       48B8AA2B11DB96010000 mov      rax, 0x196DB112BAA
       0FB600               movzx    rax, byte  ptr [rax]
       C3                   ret      
 

; Assembly listing for method Prog:ReadInt():int:this
       48B8AC2B11DB96010000 mov      rax, 0x196DB112BAC
       8B00                 mov      eax, dword ptr [rax]
       C3                   ret      


; Assembly listing for method Prog:IsWhiteSpace():bool:this
       33C0                 xor      eax, eax
       F6057723AD5E80       test     byte  ptr [(reloc 0x7ffadf84d5f0)], 128
       0F95C0               setne    al
       C3                   ret      

New codegen:

; Method Prog:ReadByte():ubyte
       B803000000           mov      eax, 3
       C3                   ret      


; Method Prog:ReadInt():int
       B805060708           mov      eax, 0x8070605
       C3                   ret      


; Method Prog:IsWhiteSpace():bool
       B801000000           mov      eax, 1
       C3                   ret     

TODO:

  • Add tests
  • Figure out why it didn't kick in for NativeAOT

@ghost ghost assigned EgorBo Nov 23, 2022
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Nov 23, 2022
@ghost
Copy link

ghost commented Nov 23, 2022

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

Closes #64823

static ReadOnlySpan<byte> RVA => new byte[] { 1, 2, 3, 4, 5, 6, 7, 8 };

// Case1: read byte at a const position (2):
byte ReadByte() => RVA[2];


// Case2: Read Int32 at the give const offset (4)
int ReadInt() =>
    Unsafe.ReadUnaligned<int>(
        ref Unsafe.Add(ref MemoryMarshal.GetReference(RVA), 4));


// Case3: IsWhiteSpace is implemented using RVA table under its hood
bool IsWhiteSpace() => char.IsWhiteSpace(' ');

Previous codegen:

; Assembly listing for method Prog:ReadByte():ubyte:this
       48B8AA2B11DB96010000 mov      rax, 0x196DB112BAA
       0FB600               movzx    rax, byte  ptr [rax]
       C3                   ret      
 

; Assembly listing for method Prog:ReadInt():int:this
       48B8AC2B11DB96010000 mov      rax, 0x196DB112BAC
       8B00                 mov      eax, dword ptr [rax]
       C3                   ret      


; Assembly listing for method Prog:IsWhiteSpace():bool:this
       33C0                 xor      eax, eax
       F6057723AD5E80       test     byte  ptr [(reloc 0x7ffadf84d5f0)], 128
       0F95C0               setne    al
       C3                   ret      

New codegen:

; Method Prog:ReadByte():ubyte
       B803000000           mov      eax, 3
       C3                   ret      


; Method Prog:ReadInt():int
       B805060708           mov      eax, 0x8070605
       C3                   ret      


; Method Prog:IsWhiteSpace():bool
       B801000000           mov      eax, 1
       C3                   ret     

TODO:

  • Add tests
  • See why we can't obtain FieldSeq* in case of NativeAOT in VN
Author: EgorBo
Assignees: EgorBo
Labels:

area-CodeGen-coreclr

Milestone: -

@EgorBo EgorBo marked this pull request as ready for review November 24, 2022 16:59
@EgorBo
Copy link
Member Author

EgorBo commented Nov 25, 2022

@jkotas does VM side look good otherwise?

@jkotas
Copy link
Member

jkotas commented Nov 26, 2022

VM/AOT side looks good.

EgorBo and others added 2 commits November 26, 2022 11:46
…foImpl.ReadyToRun.cs

Co-authored-by: Jan Kotas <jkotas@microsoft.com>
@EgorBo
Copy link
Member Author

EgorBo commented Nov 26, 2022

@jakobbotsch @SingleAccretion when you have time can you please take a look at the JIT side? A couple of notes:
I enabled GTF_ICON_STATIC_HDL local propagation for AOT - I basically repeated Andy's change here (thanks to Jakob who found that change) - just a few negative diffs in the crossgen collection (see here)

I needed that because currently we can't obtain FieldSeq* out of a VN (it might be a good idea to encode that info to VN like Single suggested but it's more work for little value IMO).

The jit-diffs are small from this change (just 8 methods improved) but I plan a couple of optimizations on top of this in C#. Also, e.g. a lot of Char apis can now be folded into a constant for constant input (e.g. after inlining).

@EgorBo
Copy link
Member Author

EgorBo commented Nov 27, 2022

/azp list

@azure-pipelines

This comment was marked as resolved.

@EgorBo
Copy link
Member Author

EgorBo commented Nov 27, 2022

/azp run runtime-coreclr outerloop, runtime-coreclr jitstress, runtime-coreclr jitstressregs

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@EgorBo
Copy link
Member Author

EgorBo commented Nov 28, 2022

Fun fact: if we intrinsify BitOperations.Log2 (to fold constant input) we can fold this thing to a const 🙂:

image

@EgorBo EgorBo merged commit 3710522 into dotnet:main Nov 29, 2022
@EgorBo EgorBo deleted the fold-const-rva branch November 29, 2022 01:31
@ghost ghost locked as resolved and limited conversation to collaborators Dec 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JIT: Fold constant access to [ReadOnly]Span<T> especially for char and byte
4 participants